New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: allow the extensions to authenticate #339
Conversation
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Pull Request Dev image published: |
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Pull Request Dev image published: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested: I was able to generate a test and use explain
function of the extension
Great job, @vitaliy-guliy !
Pull Request Dev image published: |
launcher/src/trusted-extensions.ts
Outdated
} | ||
|
||
if (!extensions.length) { | ||
console.log(' > env.VSCODE_TRUSTED_EXTENSIONS does not specify any extension'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it makes sense to add the value of the variable to the log
Let's say another separator is applied by a user, then:
- I agree that
extensions
array is empty - but
env.VSCODE_TRUSTED_EXTENSIONS does not specify any extension
is NOT true - so it makes sense to clarify it in logs, this way it would help to identify and resolve the problem quickly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
User specified env.VSCODE_TRUSTED_EXTENSIONS
in his devfile.
Launcher says that env.VSCODE_TRUSTED_EXTENSIONS
is wrong.
No need here to duplicate the value.
From the other side, if you have an idea, please share it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, Launcher says that env.VSCODE_TRUSTED_EXTENSIONS is wrong.
but the launcher says nothing about:
- what value of the variable the launcher has got
- what could be wrong
- what is expected
You use here ,
as a separator for the VSCODE_TRUSTED_EXTENSIONS
env var,
but, for example, PATH
env variable uses :
as a separator
If a user uses :
as a separator (just out of habit) for the VSCODE_TRUSTED_EXTENSIONS
then:
VSCODE_TRUSTED_EXTENSIONS
is defined, it's not empty, it contains extensions- but it doesn't work and there is nothing helpful in logs - isn't it?
My propose was just clarify it in logs - like - if the logic can not handle the variable - then it should be clear:
- env variable
VSCODE_TRUSTED_EXTENSIONS
hassome value here
=> - using
,
as a separator to parse extensions => - can not parse extensions, so the value of
VSCODE_TRUSTED_EXTENSIONS
will be ignored => - the expected format of the
VSCODE_TRUSTED_EXTENSIONS
is:some-value,another-value
============
anyway I've approved your pull request yesterday, these changes are minor, it's up to you
so feel free to merge the pull request as soon as you believe it's good enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have just played with it and would say, that the cheapest way in case of errors is to print the current value of VSCODE_TRUSTED_EXTENSIONS
environment variable and explain that it should contain one or several extension identifiers divided by comma.
It's for the first step.
The next, I think we should document the functionality provided by launcher. Among them, ability to configure trusted extensions, location of Open VSX registry, webviews, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The output with one failed extension
# Configuring Trusted Extensions...
> env.VSCODE_TRUSTED_EXTENSIONS is set to [redhat.yaml,redhat.openshift,red hat.java]
> add redhat.yaml
> add redhat.openshift
> failure to add [red hat.java] because of wrong identifier
The output when the variable does not specify anything
# Configuring Trusted Extensions...
> env.VSCODE_TRUSTED_EXTENSIONS is set to [,,,]
> ERROR: The variable provided most likely has wrong format. It should specify one or more extensions separated by comma.
Pull Request Che-Code image published: |
Pull Request Dev image published: |
Pull Request Che-Code image published: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great job
@vitaliy-guliy please, add relevant documentation about the new env var
Was checked on the DevSpaces based on 3.12 @ 0dc45 #10 :: Eclipse Che Dashboard 7.82.0 @ 896c. Worked as expected! |
Signed-off-by: vitaliy-guliy <vgulyy@redhat.com>
Pull Request Dev image published: |
Pull Request Che-Code image published: |
Build 3.13 :: code_3.x/1323: Console, Changes, Git Data |
Build 3.13 :: sync-to-downstream_3.x/6391: Console, Changes, Git Data |
Build 3.13 :: get-sources-rhpkg-container-build_3.x/6353: code : 3.x :: |
What does this PR do?
It is possible to specify trusted extensions, which can authenticate using your GitHub account by specifying the extensions identifiers in
VSCODE_TRUSTED_EXTENSIONS
environment variable.env: - name: VSCODE_TRUSTED_EXTENSIONS value: "github.copilot,github.copilot-chat"
VS Code launcher will take the content of the environment variable and will create corresponding
trustedExtensionAuthAccess
section inproduct.json
Having that, the extensions can easily authenticate without prompting the user.
It is helpful especially if the extension does not provide additional options like this
What issues does this PR fix?
eclipse-che/che#22781
How to test this PR?
!Note: for the purity of the experiment it is recommended to cleanup the browser database.
(Open browser Developer tools, switch to Application tab and remove everything in
IndexedDB
storage)https://github.com/vitaliy-guliy/recommended-extensions-sample?che-editor=https://gist.githubusercontent.com/vitaliy-guliy/1d4e35e3f6be3b109d4d62948b24b8c6/raw/b975b63aaa5475d3f8442d690353c3c00c9fb864/editor.yaml
or with
https://gist.githubusercontent.com/vitaliy-guliy/baba9c6391f5dc6590b9e7902fdfa6f7/raw/11e46945583df4ccfbb070980ef84c5b993c2671/devfile.yaml?che-editor=https://gist.githubusercontent.com/vitaliy-guliy/1d4e35e3f6be3b109d4d62948b24b8c6/raw/b975b63aaa5475d3f8442d690353c3c00c9fb864/editor.yaml
Perform
GitHub: Device Authentication
Download both extensions to your laptop
GitHub.copilot-1.173.0.vsix
GitHub.copilot-chat-0.14.2024032101.vsix
Both extensions should work.
You can check
/checode/checode-linux-libc/ubi8/product.json
file as well to be sure thetrustedExtensionAuthAccess
section is added at the end of the file.Does this PR contain changes that override default upstream Code-OSS behavior?
git rebase
were added to the .rebase folder